Skip to content

Automatic Library Installation from depends field in library.properties #205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Automatic Library Installation from depends field in library.properties #205

wants to merge 25 commits into from

Conversation

aliphys
Copy link

@aliphys aliphys commented Nov 18, 2023

This PR addresses the limitation described by @per1234 in arduino-libraries/Arduino_UnifiedStorage#30 (review)

The addition of such a capability, where the action parses the library.properties file and then automatically installs the libraries found there, has been proposed but the feature was not implemented so far.

Currently, the compile-sketches action is unable to read dependancies from the depends field. I have documented this issue in #204 .

  • 🔍 Identify if library.properties is included in the sketch directory
  • ⛏️ Extract the library names from the depends key using re (limitation: only with ',' deliminator)
  • 🔧 Modified install_libraries_from_library_manager() function to install identified libraries
  • 🎉 With this PR, there should be no need to seperately specify libraries in the workflow .yml file cc @sebromero

@codecov-commenter
Copy link

codecov-commenter commented Nov 18, 2023 •
edited
Loading

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (0e82bd5) 99.81% compared to head (1ec0bae) 99.75%.

Files Patch % Lines
compilesketches/compilesketches.py 94.44% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #205      +/-   ##
==========================================
- Coverage   99.81%   99.75%   -0.06%     
==========================================
  Files           2        2              
  Lines        1608     1665      +57     
==========================================
+ Hits         1605     1661      +56     
- Misses          3        4       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aliphys aliphys marked this pull request as ready for review November 18, 2023 12:54
@aliphys aliphys mentioned this pull request Dec 6, 2023
Draft
@aliphys
Copy link
Author

aliphys commented Dec 6, 2023

@per1234 Can you approve this PR?

per1234
per1234 requested changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unit test coverage for the added code.

@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Dec 7, 2023
@aliphys
Copy link
Author

aliphys commented Feb 6, 2024

  • 🧪 Added unit tests
    • For get_dependencies_from_properties_file -> test_get_dependencies_from_properties_file_with_dependencies, test_get_dependencies_from_properties_file_without_dependencies and test_get_dependencies_from_properties_file_no_depends_key
    • For get_library_dependencies -> test_get_library_dependencies_with_properties_file and test_get_library_dependencies_without_properties_file
  • ✅ Linter and Formatting Approved by workflow

@aliphys aliphys requested a review from per1234 February 6, 2024 13:43
@aliphys
Copy link
Author

aliphys commented Feb 6, 2024

There is an error in one workflow, which I don't think is related to this PR.

[2024-02-06T13:41:02.366Z] ['error'] There was an error running the uploader: Error uploading to [https://codecov.io:](https://codecov.io/) Error: There was an error fetching the storage URL during POST: 503 - upstream connect error or disconnect/reset before headers. reset reason: connection failure
Error: Codecov: Failed to properly upload: The process '/home/runner/work/_actions/codecov/codecov-action/v3/dist/codecov' failed with exit code 255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@per1234 per1234 Awaiting requested review from per1234

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aliphys @codecov-commenter @per1234